Repository navigation
Track history session membership by entry instead of index - #1798
Merged
Merged
Conversation
History kept the current session as a single start index and overrode each list mutator to keep it in place. That could not describe reordered lists: reverse() and sort() were not overridden, repetition and some slice assignments counted persisted copies as current, and the history --run/--edit guard then replayed persisted commands without --all. History now records which entries were present when start_session() ran. Unbounded ranges, searches, and the --all guard check that membership, so moving, filtering, or repeating entries keeps persisted commands persisted and new entries current. This replaces the list mutator overrides. Membership survives copying and pickling. session_start_index is now computed as the index just past the last persisted entry and remains assignable. Add History.is_persisted().
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1798 +/- ##
==========================================
- Coverage 99.75% 99.75% -0.01%
==========================================
Files 23 23
Lines 6624 6592 -32
==========================================
- Hits 6608 6576 -32
Misses 16 16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Move the behavior changes from the History session membership fix out of Bug Fixes. Entries added after start_session() are current wherever they are placed, session_start_index is computed and its setter marks only entries already in the list, and history[session_start_index:] is unreliable after reordering, so is_persisted() is the check to use.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1795. A review of that change found that a single
session_start_indexcan't describe the session once the list is changed in place. This PR changes howHistorytracks the session.Historynow records which entries were present whenstart_session()ran. Unbounded ranges,str_search(),regex_search(), and thehistory --run/--edit--allguard all check that membership. Moving, filtering, or repeating entries therefore keeps persisted commands persisted and new entries current. This replaces the six list mutator overrides added in #1795.Problems fixed
Using 4 persisted entries (
o0–o3) followed by 2 current entries (n0,n1):reverse()o1, o0n1, n0sort()ascending by rawo2, o3n0, n1history *= 2/history += list(history)o0–o3counted as currentn0, n1copies are currenthistory --run 5:6afterreverse()--allhistory[:] = [history[1], new]right afterstart_session()newhidden as persistednewis currenthistory[:] = [n0, o1, n1]o1counted as currento1stays persistedBehavior notes
HistoryItemobject again keeps it persisted. To record a persisted command again in the current session, append its statement (history.append(history[i].statement)). This is documented inHistory.is_persisted().session_start_indexis now a computed property: the index just past the last persisted entry. Every entry from that index on is current. Assigning it still marks the firstnentries as persisted, so existing callers and tests that set it keep working.copy.deepcopy()and pickling. It is keyed by an identity wrapper rather than rawid()values, so copies and unpickled histories still recognize their persisted entries.History.is_persisted(item).Tests
tests/test_history_mutations.pywas rewritten around acurrent_session()helper. It returns{command number: raw}and checks thatspan(":"),str_search(), andregex_search()agree. It also checks thesession_start_indexrule. Expected values are hand-derived literals.reverse/sort/*=/+=, assignment into an empty session, persisted entries placed after current ones, inserts at any position, re-appending a persisted object, deepcopy/pickle, and the--allguard after reordering and repetition. Each failed onmainbefore the change.test_remove_retains_native_list_dispatchandtest_failed_straddling_replacement_preserves_history, which tested the internals of the deleted overrides. Failed mutations are still covered bytest_failed_mutation_preserves_history_and_membership.Test plan
make check(ruff, prettier, typos, ty, mypy)make test: 2924 passed, 3 skipped;cmd2/history.pyat 100% coveragemake docs-test